-
Notifications
You must be signed in to change notification settings - Fork 5
Able to use settings as context settings #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
theboxer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The support should be also extended into the modAI\API\Prompt\Vision endpoint
|
|
||
| public function post(ServerRequestInterface $request): void | ||
| { | ||
| $contextKey = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default should be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| $base = Settings::getTextSetting($this->modx, $field, 'base_prompt', $namespace, false); | ||
| $fieldPrompt = Settings::getTextSetting($this->modx, $field, 'prompt', $namespace); | ||
| $customOptions = Settings::getTextSetting($this->modx, $field, 'custom_options', $namespace, false); | ||
| $model = Settings::getTextSetting($this->modx, $field, 'model', $namespace, 'openai/gpt-4o-mini', $contextKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong arguments, instead of required you're passing model name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| $fieldPrompt = Settings::getTextSetting($this->modx, $field, 'prompt', $namespace); | ||
| $customOptions = Settings::getTextSetting($this->modx, $field, 'custom_options', $namespace, false); | ||
| $model = Settings::getTextSetting($this->modx, $field, 'model', $namespace, 'openai/gpt-4o-mini', $contextKey); | ||
| $temperature = (float)Settings::getTextSetting($this->modx, $field, 'temperature', $namespace , 0, $contextKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong arguments, the required one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| $customOptions = Settings::getTextSetting($this->modx, $field, 'custom_options', $namespace, false); | ||
| $model = Settings::getTextSetting($this->modx, $field, 'model', $namespace, 'openai/gpt-4o-mini', $contextKey); | ||
| $temperature = (float)Settings::getTextSetting($this->modx, $field, 'temperature', $namespace , 0, $contextKey); | ||
| $maxTokens = (int)Settings::getTextSetting($this->modx, $field, 'max_tokens', $namespace, 0, $contextKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong arguments, the required one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| $maxTokens = (int)Settings::getTextSetting($this->modx, $field, 'max_tokens', $namespace, 0, $contextKey); | ||
| $output = Settings::getTextSetting($this->modx, $field, 'base_output', $namespace, false, $contextKey); | ||
| $base = Settings::getTextSetting($this->modx, $field, 'base_prompt', $namespace, false, $contextKey); | ||
| $fieldPrompt = Settings::getTextSetting($this->modx, $field, 'prompt', $namespace, false, $contextKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the prompt for field should be required, that's the whole reason for the generate button on those fields, that it'll get prompt from a system setting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| * @return string|null | ||
| */ | ||
| private static function getOption(modX $modx, string $namespace, string $field, string $area, string $setting): ?string | ||
| private static function getOption(modX $modx, string $namespace, string $field, string $area, string $setting, string $contextKey = 'web'): ?string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default value for $contextKey should be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| $handler = $modx; | ||
|
|
||
| if (!empty($contextKey)) { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete the empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| * @throws RequiredSettingException | ||
| */ | ||
| public static function getTextSetting(modX $modx, string $field, string $setting, string $namespace = 'modai', bool $required = true): ?string | ||
| public static function getTextSetting(modX $modx, string $field, string $setting, string $namespace = 'modai', bool $required = true, string $contextKey = ''): ?string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default for $contextKey should be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
With this change we can now use the modai system settings as context setting.
This helps with multi language websites to have seperate prompts for each context.